Skip to content

Initial clustered lights implementation (tiled clustering only) #16866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 39 commits into
base: master
Choose a base branch
from

Conversation

matanui159
Copy link
Contributor

@matanui159 matanui159 commented Jul 9, 2025

Remaining issues

  • causes issues with the default physical material falloff. was considering preventing this in the shader but decided to leave it so that if a user wants to use physical falloff they still can as long as the adjust the light ranges accordingly (and the default light range is absolutely large enough for physical falloff)
  • there's no depth clipping (to prevent lights disappearing when too close or too far from the camera). this does also mean lights behind the camera will be marked as "contributing" to pixels in front of the camera (their light meshes get flipped). couldn't find an easy and clean solution to only prevent clipping if the light is within range and once depth clustering is implemented it will fix this issue (since any lights behind the camera won't end up in any depth slice)

Implementation Details

When this work is more complete ill move all this over to the official docs, but for now just using this PR as a rough set of notes/thoughts/todos

The implementation is mainly inspired by these CoD slides: https://advances.realtimerendering.com/s2017/2017_Sig_Improved_Culling_final.pdf

The main idea is to split the clustering into separate X/Y "tiled" clustering AND a Z "depth" clustering, and then checking if the lights is in both the tile and the depth slice.

Tiled Clustering

This uses the rendering pipeline to draw meshes for each light and writes bits out to a light mask texture if there are pixels infront of the depth buffer (effectively finding lights that intersect the depth buffer).

The render target with the light mask texture and depth buffer has a resolution equal to the amount of tiles.

Depth pre-pass (removed)

All the scene geometry is rendered to a low resolution render target. Currently this is repeated per ClusteredLight, in future these depth texture might be able to get shared

Stencil Pass (removed)

Draws all the light geometry with depthFunction=GREATER and sets stencil bits to 1.

This is combined with the next pass to only mark pixels that contain light geometry both behind AND infront of the depth buffer.

However this ended up causing issues due to the low-resolution aliased depth buffer where stencil bits weren't set for the edges of meshes.
image

In future we could maybe get around this by doing a post-process pass over the depth pre-pass to expand the max depth values out by one tile. Although for now this just adds extra complexity and we probably don't really need this pass.

Light Mask Pass

Each light mesh is drawn with a different index and any fragment pixels which pass the depth test (meaning the light is in front of the depth pre-pass) will set its bit in the light mask texture with an atomic OR.

Atleast thats the plan on WebGPU which isn't implemented yet. WebGL doesn't have support for storage textures or atomic operations, so we instead use a float buffer with additive blending.

To prevent accuracy issues with floating point values we ensure the mask doesn't go above the bits in the floating-point fraction, which limits us to 23 lights per clusteredlight on most systems.

Lighting pass

This light mask is then queried bit-by-bit in the lighting calculation to figure out which lights are in that "tile"

Light Meshes

All the light meshes are just spheres. To reduce the pixels set for spotlights they are modified in the vertex shader so positions on the sphere that are outside the spotlight angle are set to 0,0,0. This causes the sphere to end up having a more cone shape

Depth Clustering

TODO. not currently implemented. will be implemented in a different PR

CoD seems to do this in CPU which makes sense since its only a min/max index per slice so can be cheaply calculated and uploaded.

Not Implemented / Broken

  • PBR materials
  • I have code for point lights but its untested. I feel like they likely won't work rn
  • moving the camera too close or too far from the lights make them disappear
    • this is an issue with the camera intersecting with the light geometry, on WebGPU this can be fixed by disabling back-face culling but on WebGL this will cause more issues due to its additive blending
    • on WebGL we could test if the light mesh intersects with the camera and draw back faces instead
  • spot lights that point in any other direction but down
    • to make the light mesh vertex shader simpler we assume a direction of down. this will have to get changed or (better) we just rotate the world matrix by the light direction
  • lights with a default range. trying to scale the light geometry by Number.MAX_VALUE seems to break it
    • maybe we should clamp it?
  • WebGPU support
  • Using instancing for the light meshes
  • make the amount of tiles configurable
    • I don't know if this should be an options to specify the number of tiles or to specify the size of each tile
  • figure out what to do for transferToNodeMaterialEffect

I don't plan to support these kinda lights:

  • anything that requires textures like IES textures or shadowed lights
  • anything other than point/spot lights
  • lights using anything other than the default falloff

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 9, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 10, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

3 similar comments
@bjsplat
Copy link
Collaborator

bjsplat commented Jul 11, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 14, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 16, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@matanui159
Copy link
Contributor Author

Some more resources about the subject:

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 17, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

19 similar comments
@bjsplat
Copy link
Collaborator

bjsplat commented Jul 17, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 17, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 17, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 18, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 21, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 22, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 23, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 23, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 24, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 24, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 24, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 24, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 25, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 25, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 29, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 29, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 30, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 30, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 30, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 30, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 30, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 30, 2025

@matanui159
Copy link
Contributor Author

  • I think a test is needed to verify that this also works when using custom render targets using cameras other than the main camera.

    • Actually, this use case isn't implemented; it's something we'll need to make work.

It should work as long as the other cameras are added to the active camera list. Not sure the best way to support any render target would be, especially if we don't want to run it for things like shadow generation.

@matanui159
Copy link
Contributor Author

Subsequent follow up should be NME, and FrameGraphs

There's still the depth clustering PR that I will be opening after this gets merged.

@matanui159
Copy link
Contributor Author

btw these are the main playgrounds I've been using to test:
http://localhost:1338/#20OAV9#15356
http://localhost:1338/#CSCJO2#3

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 11, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 11, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 11, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 11, 2025

@Popov72
Copy link
Contributor

Popov72 commented Aug 12, 2025

  • I think a test is needed to verify that this also works when using custom render targets using cameras other than the main camera.

    • Actually, this use case isn't implemented; it's something we'll need to make work.

It should work as long as the other cameras are added to the active camera list. Not sure the best way to support any render target would be, especially if we don't want to run it for things like shadow generation.

The use case I'm speaking about above is the one where we create a RenderTargetTexture, with a RenderTargetTexture.activeCamera different from scene.activeCamera and with different viewing parameters:

http://localhost:1338/#20OAV9#15378

We can fix it by generating the mask texture in RTT.onBeforeBindObservable:

http://localhost:1338/#20OAV9#15377

We could add the code in the RenderTargetTexture.constructor, but I may miss something...

cc @sebavan

@Popov72
Copy link
Contributor

Popov72 commented Aug 12, 2025

Subsequent follow up should be NME, and FrameGraphs

There's still the depth clustering PR that I will be opening after this gets merged.

Note that for WebGPU, you will probably need to sort the light on the GPU, otherwise depth buffer management will become quite complicated, as updating a storage buffer from the CPU to the GPU does not happen in the same timeframe as the execution of graphics commands.

To illustrate this, if you have two renderings from two cameras that follow each other, we currently (WebGPU) proceed as follows:

  • camera 1:
    • generate a mask in StorageBufferMask
    • render objects with StorageBufferMask
  • camera 2:
    • generate a mask in StorageBufferMask
    • render objects with StorageBufferMask

This works because StorageBufferMask is updated by some graphics commands that are executed in the same command stream as the rendering commands (they are all executed when we do device.queue.submit(commands)).

Now, if we add depth clustering and perform sorting on the CPU, we will need to copy the result to a storage buffer:

  • camera 1:
    • generate a mask in StorageBufferMask
    • generate depth information in a CPU array
    • copy the CPU array to StorageBufferDepth
    • render objects with StorageBufferMask and StorageBufferDepth
  • camera 2:
    • generate a mask in StorageBufferMask
    • generate depth information in a CPU array
    • copy the CPU array to StorageBufferDepth
    • render objects with StorageBufferMask and StorageBufferDepth

The problem is that the copy from the CPU to the storage buffer is done immediately, not when we execute device.queue.submit(commands). As a result, the step “render objects with StorageBufferMask and StorageBufferDepth” for camera 1 and camera 2 will use the same data for StorageBufferDepth, namely the data generated for camera 2.

@Popov72
Copy link
Contributor

Popov72 commented Aug 13, 2025

I ran some more tests and encountered a few problems:

image image image image The footprint uses a large part of the mask texture, whereas it should only use a small part to be effective.

@matanui159
Copy link
Contributor Author

Spot management is rather inefficient, as it uses a disk to generate the mask:

Couldn't really find an easy way to adjust the disc to be shaped to a cone depending on angle its looked at. So yeah currently it just does the full disc. Depth clustering will help with this since further away or closer pixels will ignore the light

@matanui159
Copy link
Contributor Author

The two where the mask is too large is I think is because we don't do any depth pre-pass so we can't limit the light to the intersection of the plane since the plane could be closer to the screen in which case it would be larger.

@matanui159
Copy link
Contributor Author

as for the last one.....
image
istg I swear I get these meshes perfect and then there's still some issue... seems like even tho the vertices are rounded to a tile border, the angle between them is not. not sure currently what a good fix for this would be but im open to ideas

@Popov72
Copy link
Contributor

Popov72 commented Aug 14, 2025

The two where the mask is too large is I think is because we don't do any depth pre-pass so we can't limit the light to the intersection of the plane since the plane could be closer to the screen in which case it would be larger.

Indeed, I should have drawn a sphere at the light location, with a radius = range of the light (http://localhost:1338/#20OAV9#15389) to see what the mask dimensions should look like:

image

We can see it's ok.

as for the last one..... istg I swear I get these meshes perfect and then there's still some issue... seems like even tho the vertices are rounded to a tile border, the angle between them is not. not sure currently what a good fix for this would be but im open to ideas

It looks like the problem comes from the fact that the disk we rasterize to find the clusters the light intersects with is perspective projected, meaning it will be smaller than what it should be.

See http://localhost:1338/#20OAV9#15396:

image

In this example, the yellow mesh is a sphere which represents the light (positionned at 0,0,0) and the red mesh is a disk mesh which represents the disk that will be rasterized to find the clusters the light intersects with. I took the disk creation from ClusteredLight.ts and scaled it by the range of the light. The disk is positionned at the light location. If I didn't make any mistake, it should represent what is done by the lightProxy shader.

As you can see, the red disk is smaller than the yellow disk, because of the perspective transform. To fix it, I think you will have to calculate an offset to move the disk nearer to the camera (in the camera coordinate system, so a negative offset to viewPosition.z), so that the disk exactly fits the yellow sphere.

@matanui159
Copy link
Contributor Author

matanui159 commented Aug 14, 2025

As you can see, the red disk is smaller than the yellow disk, because of the perspective transform. To fix it, I think you will have to calculate an offset to move the disk nearer to the camera (in the camera coordinate system, so a negative offset to viewPosition.z), so that the disk exactly fits the yellow sphere.

Ended up doing this but as a square. Tried keeping the disc with it but the maths was already getting complicated enough trying to get +/- X/Y rotations to the sphere horizons that I didn't bother trying to figure it out for each disc point. That GitHub webgl project already uses squares too with repositioning to sphere horizons

I think ideally the best shape is sphere but that causes issues with WebGL additive blending (even with back face culling since any modifications in the vertex shader like rounding to tile corners can cause unintentional overlap which I never was able to solve)

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 14, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 14, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 14, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 14, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 15, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 15, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 15, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 15, 2025

Copy link
Contributor

@Popov72 Popov72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the calculations are quite complex in proxyVertex.fx, it would be useful to have a debugging method in ClusteredLightContainer that would render the masks (alpha blended) on the current output screen. This would make it much easier to verify that everything is working correctly by simply walking through the Sponza scene and checking that the masks match the projections of the light shape.

In addition, this code added to RenderTargetTexture.constructor should enable support for cluster lights in custom render targets:

    this.onBeforeBindObservable.add(() => {
        for (const light of scene.lights) {
            if (light.getTypeID() === BABYLON.LightConstants.LIGHTTYPEID_CLUSTERED && light.isSupported) {
                light._updateBatches().render();
            }
        }
    });
  • add a visu test with an RTT.

Thanks for all your hard work on this awesome new feature!

} else {
// Only clear the texture on WebGL
}
engine.clear({ r: 0, g: 0, b: 0, a: 1 }, true, false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in the else above.

@@ -6,7 +6,38 @@
vec4 diffuse{X} = light{X}.vLightDiffuse;
#define CUSTOM_LIGHT{X}_COLOR // Use to modify light color. Currently only supports diffuse.

#ifdef PBR
#if defined(PBR) && defined(CLUSTLIGHT{X}) && CLUSTLIGHT_BATCH > 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this offline with @sebavan, you should add a big warning at the top of the file stating that if anything is updated in this file regarding PBR, pbrClusteredLightingFunctions.fx will likely need to be updated accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants